ENH/SE: don't deadreckon position #218
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds a deadreckon flag to raw_to_timeseries, updates tests and example scripts to exercise both modes, and adjusts example YAML to use surface latitude/longitude (Lat/Lon) instead of dead-reckoned fields.
- Introduce
deadreckon=Falseparameter inraw_to_timeseriesto ignore underwater dead-reckoning jumps - Update tests and example scripts to pass
deadreckonflag explicitly - Change example deployment YAML to reference
Lat/Lonas sources for latitude and longitude
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| tests/test_pyglider.py | Pass deadreckon=True to raw_to_L0timeseries calls |
| tests/example-data/.../process_deploymentRealTime.py | Add deadreckon=False to example script invocation |
| tests/example-data/.../deploymentRealtime.yml | Switch latitude/longitude source: keys to Lat/Lon |
| pyglider/seaexplorer.py | Implement deadreckon logic for filtering lat/lon |
Comments suppressed due to low confidence (2)
tests/test_pyglider.py:100
- [nitpick] There’s no test covering the default
deadreckon=Falsepath—adding a case to confirm that underwater positions are set to NaN whendeadreckonis not passed would help ensure correct behavior.
seaexplorer.raw_to_L0timeseries(
tests/test_pyglider.py:101
- Verify that
raw_to_L0timeseriesactually accepts adeadreckonkeyword argument; if not, either update that function or change this test to callraw_to_timeseriesdirectly.
rawncdir, l0tsdir, deploymentyaml_raw, kind='raw', deadreckon=True
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a deadreckon option to raw_to_timeseries that disables dead-reckoned positions and only uses valid surface fixes for lat/lon. It also updates tests, example scripts, and YAML to pass the new flag and adjust coordinate sources.
- Add
deadreckonkeyword (defaultFalse) toraw_to_timeseriesand implement helper functions to drop and forward‐fill coordinates. - Update unit tests and example workflows to pass the
deadreckonflag. - Change example deployment YAML to use
Lat/Lonsources instead ofNAV_LATITUDE/NAV_LONGITUDE.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/test_pyglider.py | Updated calls to include deadreckon=True in timeseries tests |
| tests/example-data/.../process_deploymentRealTime.py | Pass deadreckon=False to raw_to_timeseries in example script |
| tests/example-data/.../deploymentRealtime.yml | Switch lat/lon sources from NAV_LATITUDE/NAV_LONGITUDE to Lat/Lon |
| pyglider/seaexplorer.py | Added deadreckon parameter, _forward_fill & _drop_if helpers, and coordinate‐dropping logic |
Comments suppressed due to low confidence (2)
tests/test_pyglider.py:101
- The test calls
raw_to_L0timeserieswithdeadreckon=True, butraw_to_L0timeseriesmay not accept that parameter. Either extend its signature to pass throughdeadreckonor update the test to callraw_to_timeseriesdirectly.
outname = seaexplorer.raw_to_L0timeseries(rawncdir, l0tsdir, deploymentyaml_raw, kind='raw', deadreckon=True)
tests/test_pyglider.py:150
- Similarly, this call to
raw_to_L0timeseriespassesdeadreckon=Truebut the function signature doesn't include it. Consider updating that function or test accordingly.
outname_interp_raw = seaexplorer.raw_to_L0timeseries(rawncdir, l0tsdir_interp_raw, interp_yaml, kind='raw', deadreckon=True)
|
|
||
|
|
||
| def _drop_if(gli, todo='Lat', condit='DeadReckoning', value=1): | ||
| """Drop Lat if DeadReckoning is 1""" |
There was a problem hiding this comment.
[nitpick] Docstring for _drop_if is hardcoded to 'Lat' and 'DeadReckoning'; generalize to mention parameters todo, condit, and value.
| """Drop Lat if DeadReckoning is 1""" | |
| """ | |
| Drop values in the column specified by `todo` if the column specified by `condit` equals `value`. | |
| Parameters: | |
| gli (polars.DataFrame): The input DataFrame. | |
| todo (str): The name of the column to modify. | |
| condit (str): The name of the column to check the condition against. | |
| value (any): The value to compare against in the `condit` column. | |
| Returns: | |
| polars.DataFrame: The modified DataFrame with updated values in the `todo` column. | |
| """ |
Co-authored-by: Copilot <[email protected]>
|
@richardsc can you guys test this when you get a chance? |
|
Hi @jklymak -- sorry for being slow. @clayton33 and I were re-running some of our missions to see if this behaved as expected. It does! (though as a heads-up we will be working through some other issues we found, in the case where a user might actually want to keep and use some of these nav variables, such as deadReckoning, navState, etc). |
This closes #4
For
raw_to_timeseriesI've added a new kwargdeadreckon=Falsewhich ignores the dead reckoning, and only has valid lat and Lon when the glider is on the surface.This is the default, but if you use
NAV_LATITUDEandNAV_LONGITUDEin the yaml, you probably want to change toLatandLonrespectively to pick up the lat and Lon in the glider file.